Skip to content

CCD-6626 CCD is retaining value even after user has manually removed it from UI#2840

Open
markdathornehmcts wants to merge 4 commits intomasterfrom
CCD-6626_202603
Open

CCD-6626 CCD is retaining value even after user has manually removed it from UI#2840
markdathornehmcts wants to merge 4 commits intomasterfrom
CCD-6626_202603

Conversation

@markdathornehmcts
Copy link
Copy Markdown
Contributor

JIRA link (if applicable)

https://tools.hmcts.net/jira/browse/CCD-6626

Change description

CCD is retaining values on Check Your Answers which were removed by the user on a page.

Does this PR introduce a breaking change? (check one with "x")

[ ] Yes
[*] No

@@ -76,12 +75,24 @@ public CaseDetails createNewCaseDetails(String caseTypeId, String jurisdictionId
* @return <code>Optional&lt;CaseDetails&gt;</code> - CaseDetails wrapped in Optional
*/
public CaseDetails populateCurrentCaseDetailsWithEventFields(CaseDataContent content, CaseDetails caseDetails) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes look OK, but I'm not sure how SSCS invokes this method. I think it might be via the CaseDataValidatorController or CaseDetailsEndpoint endpoints, but I can't identify route. It might be something from XUI.

I would suggest renaming the method it's now populating with whatever is returned by getData() as well as getEventData(), but only do this if the change fixes the issue. Try it out as a proof of concept first.

Copy link
Copy Markdown
Contributor

@paulridout paulridout left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes look OK, but would recommend trying it out as a proof of concept first.

Copy link
Copy Markdown
Contributor

@padmapriyanalam padmapriyanalam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

change looks ok. I agree with Paul. Could this be tested in a lower environment first to ensure that the change fixes the issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants